Skip to content

ffi: add new methods and error codes#62762

Open
ShogunPanda wants to merge 1 commit intonodejs:mainfrom
ShogunPanda:ffi-followup
Open

ffi: add new methods and error codes#62762
ShogunPanda wants to merge 1 commit intonodejs:mainfrom
ShogunPanda:ffi-followup

Conversation

@ShogunPanda
Copy link
Copy Markdown
Contributor

@ShogunPanda ShogunPanda commented Apr 15, 2026

As promised to @justjake, also the following followups requested in #62072 (comment):

  • Added ffi.exportArrayBuffer(...) and ffi.exportArrayBufferView(...).
  • Added raw pointer helper ffi.getRawPointer(source) (Buffer | ArrayBuffer | ArrayBufferView), with strong “unsafe/dangerous” docs and tests.
  • Moved byte-export copying to native code (exportBytes) to avoid extra JS Buffer wrapper allocations.

@ShogunPanda ShogunPanda requested review from bengl and jasnell April 15, 2026 18:28
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 15, 2026

Review requested:

  • @nodejs/config
  • @nodejs/gyp
  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 15, 2026
@ShogunPanda ShogunPanda mentioned this pull request Apr 15, 2026
Closed
@justjake
Copy link
Copy Markdown

justjake commented Apr 15, 2026 •
edited
Loading

Great! Thank you. It looks like I won't need my own unsafe-pointer once node:ffi gets stabilized 🎉 😄

@justjake justjake mentioned this pull request Apr 15, 2026
Open
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 60.15326% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.66%. Comparing base (3f52482) to head (927da3b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/ffi/data.cc 41.17% 45 Missing and 15 partials ⚠️
src/node_ffi.cc 46.80% 24 Missing and 1 partial ⚠️
src/ffi/types.cc 59.57% 19 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #62762      +/-   ##
==========================================
- Coverage   91.55%   89.66%   -1.89%     
==========================================
  Files         356      706     +350     
  Lines      149601   218288   +68687     
  Branches    23395    41773   +18378     
==========================================
+ Hits       136967   195728   +58761     
- Misses      12371    14455    +2084     
- Partials      263     8105    +7842
Files with missing lines Coverage Δ
lib/ffi.js 96.56% <100.00%> (+0.71%) ⬆️
src/node_errors.h 86.48% <ø> (ø)
src/node_ffi.h 72.72% <ø> (ø)
src/node_options.h 98.00% <100.00%> (ø)
src/ffi/types.cc 59.82% <59.57%> (ø)
src/node_ffi.cc 69.04% <46.80%> (ø)
src/ffi/data.cc 71.06% <41.17%> (ø)

... and 465 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@meixg
Copy link
Copy Markdown
Member

meixg commented Apr 16, 2026

LGTM
One small API thought: would it be better to expose this as something like getRawReference() instead of getRawPointer(), returning both the pointer and byteLength? For byte sources, the pointer is usually
meant to be used together with its length.

@ShogunPanda
Copy link
Copy Markdown
Contributor Author

ShogunPanda commented Apr 16, 2026

@meixg I don't think it is acually necessary. When using the function you already own the source (getRawPointer(buffer)) so you can call the byteLength directly, isn't it?

@ShogunPanda ShogunPanda changed the title ffi: added new methods and error codes ffi: add new methods and error codes Apr 16, 2026
@ShogunPanda
lib: add new methods and error codes
1fee1dd 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@meixg meixg meixg approved these changes

@bengl bengl Awaiting requested review from bengl

@jasnell jasnell Awaiting requested review from jasnell

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ShogunPanda @nodejs-github-bot @justjake @meixg